[security] Add MCP elicitation for secure preview token handling - #57
Open
mattpodwysocki wants to merge 18 commits into
Open
[security] Add MCP elicitation for secure preview token handling#57mattpodwysocki wants to merge 18 commits into
mattpodwysocki wants to merge 18 commits into
Conversation
- Implement token elicitation to keep tokens out of chat history - Users can provide, create, or auto-create preview tokens - Add session-level token storage to avoid repeated prompts - Support URL-restricted tokens for enhanced security - Maintain backward compatibility with direct token provision - Update README with security best practices Security improvements: - Preview tokens no longer appear in chat history via elicitation - Users can create URL-restricted tokens inline - Token caching reduces friction while maintaining security Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Critical security fix for PreviewStyleTool: - Add `public: true` flag to token creation API request body - Validate that created tokens start with 'pk.' prefix - Prevent accidental creation of secret tokens (sk.*) which should never be exposed in browser URLs This ensures preview URLs always use public tokens that can be safely shared in preview URLs without security risk. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Root cause: The Mapbox Tokens API automatically determines token type (public vs secret) based on the SCOPES requested, not an explicit parameter. Problem: - We were requesting 'styles:download' which is a SECRET scope - This forced the API to create a secret token (sk.*) instead of public (pk.*) - Secret tokens cannot be safely exposed in browser URLs Solution: - Changed scopes to only public scopes: ['styles:read', 'styles:tiles', 'fonts:read'] - These are sufficient for preview URLs and guarantee public token creation - Removed the unsupported 'public: true' parameter - Updated comments to explain the scope selection rationale Testing: Verified in MCP Inspector that auto-create now produces pk.* tokens Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
According to the MCP specification, servers must verify that the client supports elicitation capability before attempting to use elicitInput(). Changes: - Added client capability check before calling elicitPreviewToken() - Returns clear error message if client doesn't support elicitation - Suggests providing accessToken parameter directly as fallback - Prevents "Method not found" errors when client lacks capability This fixes the issue where tools using elicitation would fail on clients that don't advertise elicitation support in their capabilities. Reference: https://modelcontextprotocol.io/specification/2025-11-25/client/elicitation Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Added documentation to clarify that MCP elicitation support varies by client: - MCP Inspector has full support for secure token elicitation - Claude Desktop does not support elicitation yet, but Claude intelligently falls back to offering token creation via create_token_tool - Other clients should check their documentation for elicitation support Changes: - Added "Note on MCP Elicitation Support" in Quick Start section - Updated PreviewStyleTool description with client-specific behavior - Clarified that tokens appear in chat history when elicitation is unavailable - Added visual indicators (✅/⚠️ ) for support status This helps users understand expected behavior based on their MCP client. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Confirmed that Cursor and VS Code both have full MCP elicitation support. Updated README to accurately reflect support status: ✅ Full support: - MCP Inspector - Cursor - VS Code (with Copilot)⚠️ Not yet supported: - Claude Desktop (falls back to create_token_tool) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Created comprehensive bug report for Goose's MCP elicitation timing issue where forms display after timeout instead of during tool execution. Added: - docs/goose-elicitation-bug-report.md - Detailed bug report for Goose team with reproduction steps, expected vs actual behavior, technical details, and suggested fix - Updated README to document Goose's known elicitation bug with link to bug report in both Quick Start and PreviewStyleTool sections Bug Summary: Goose advertises elicitation capability but displays forms after tool execution completes/times out, preventing user input. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Updated bug report and README to reference the filed GitHub issue: aaif-goose/goose#6471 This allows users and developers to track the bug status directly with the Goose team. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Bug is now tracked on GitHub at aaif-goose/goose#6471 No need to maintain a duplicate markdown file in the repo. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Added comprehensive test coverage for the new elicitation features: Token Storage Tests (test/utils/tokenElicitation.test.ts): - Store and retrieve tokens by username - Return undefined for non-existent username - Overwrite existing tokens - Store tokens for multiple users independently - Clear specific username token - Clear all tokens - Handle edge cases (empty string, special characters) PreviewStyleTool Elicitation Tests: - Error when no accessToken and no server token - Backward compatibility when accessToken provided directly Test Results: All 527 tests pass (12 new tests added) These tests ensure the elicitation feature works correctly and maintains backward compatibility with existing usage patterns. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Tested preview_style_tool directly via MCP and confirmed that Claude Code does not advertise elicitation capability. The tool correctly returns the error message we designed for clients without elicitation support. Updated README to reflect: - Claude Code:⚠️ Not yet supported (provide accessToken directly) - Grouped with Claude Desktop in the "not yet supported" category This was confirmed by calling the tool through the registered MCP server and observing the capability check work as expected. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Made accessToken optional and added useCustomToken parameter - Integrated elicitation flow with capability checks - Added token creation/listing methods with minimal public scopes - Session caching via shared previewTokenStorage - Added 2 elicitation behavior tests - Updated README with security-focused documentation - All 529 tests pass
jussi-sa
previously approved these changes
Mar 25, 2026
…citation # Conflicts: # src/tools/preview-style-tool/PreviewStyleTool.ts # src/tools/style-comparison-tool/StyleComparisonTool.ts # test/tools/style-comparison-tool/StyleComparisonTool.test.ts
…licitation - Reject token creation up front when the server's access token is a temporary tk.* token (used by the hosted MCP endpoint), instead of letting the Mapbox API round-trip fail. The elicitation dialog now omits "create"/"auto-create" in that case and only offers "provide an existing token". - Move token-listing/creation off raw fetch() onto the shared HttpPipeline (constructor-injected httpRequest), consistent with other Mapbox API tools. - Document the hosted-endpoint limitation in README and CHANGELOG.
… message The tk.* prefix check only catches a literal Mapbox temporary token supplied directly (e.g. MAPBOX_ACCESS_TOKEN=tk...). It does not detect the hosted MCP endpoint's lack of tokens:write: that deployment passes through its own access token, which isn't tk.*-shaped, so the guard never fires there. Corrected the doc comments, README, and CHANGELOG, which previously stated this as a general fact about the hosted endpoint's token shape. Since the guard can't see that case, createPreviewToken() now appends a scope/permission hint to whatever error the Tokens API returns on a 401/403 (or a message containing "scope"/"permission"), steering back to "provide an existing token" instead of leaving the caller to interpret a bare API error.
…ocol Every existing elicitation test fakes tool['server'] directly and never proves the SDK's own capability negotiation and request/response plumbing works end to end. This spins up a real Streamable HTTP MCP server (session-scoped, one McpServer/transport pair per Mcp-Session-Id) and drives it with a real Client that answers elicitation/create requests, modeled on hosted-mcp-server's own request handling (bearer token from Authorization attached to the raw request as .auth). Covers, fully offline (httpRequest mocked, no real network calls): - tk.* server token: dialog trims to ["provide"], Tokens API never called - non-tk.*-shaped token (the hosted-endpoint case): dialog offers all three choices, auto-create fails against a mocked 403 and the error includes the scope/permission hint - non-tk.*-shaped token: auto-create succeeds end to end - style_comparison_tool gets the same tk.* trimming as preview_style_tool Building this surfaced a real, separate finding worth a follow-up: a first attempt used a fresh McpServer per HTTP request (the "stateless" pattern both mcp-server's scripts/dev-http-server.ts and hosted-mcp-server's src/routes/mcp.ts use), and every elicitation call failed with "client does not support elicitation" regardless of what the client declared. Server#getClientCapabilities() is only ever set on whichever Server instance processes the client's initialize request; a fresh Server per request means the tools/call request's instance never saw that handshake. Documented in the harness's doc comment; not otherwise addressed here since it isn't this PR's tool code.
buildTransport() called mcpServer.connect(transport) without awaiting it, then returned the transport for immediate use by the very next handleRequest() call. Locally the connect() promise happened to settle before that mattered; under CI's different scheduling, the first request (the client's initialize) sometimes raced ahead of the server's own wiring, and the request never got a response — observed as the style_comparison_tool test timing out after 60s with "MCP error -32001: Request timed out" while the other three tests in the same file passed. Made buildTransport async and awaited it at the call site. Ran the suite 8x locally with no failures after the fix (it never reproduced locally to begin with, consistent with a narrow scheduling-dependent race rather than a logic bug in the tools themselves).
Both tools' fallback error (shown when the client lacks the elicitation capability) named Claude Desktop and Claude Code as example clients that support MCP elicitation — exactly backwards. Per the README's own support matrix, those two are the ones that *don't* support it; only MCP Inspector, Cursor, and VS Code do. Confirmed live in Claude Desktop, where the model was relaying this text almost verbatim while correctly working around it by asking the user for a pk. token directly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements MCP elicitation for secure token management in
preview_style_toolandstyle_comparison_tool, following the principle of least privilege. Elicitation ensures that only minimal-scope public tokens (pk.) appear in preview URLs, while your powerful server token (sk.) stays secure.Update: Rebase onto
main, HttpPipeline, and Hosted Endpoint HandlingThis branch sat for a while, so it's been rebased onto current
mainand hardened before merge (see details below for the full file list):main- resolved conflicts against the path-traversal fix (encodeURIComponentaround username/styleId), the sharedstyleIdSchema, the now-unconditional MCP-UImeta/CSP block, and_meta.viewUUID.style_comparison_toolelicitation (described throughout this PR) is now actually implemented to match the description below - it wasn't in the original commits.fetch()- token listing/creation now go through the sharedHttpPipeline(constructor-injectedhttpRequest), matching every other Mapbox-API-calling tool in this repo, and consolidated the duplicated listing/creation logic from both tools into shared functions intokenElicitation.ts. This is a constructor signature change for direct consumers - see Breaking Changes.tokens:writeon the caller's own access token. A literal Mapbox temporary token (tk.*) never has that scope, soisTemporaryServerToken()detects that shape up front and trims "create"/"auto-create" from the elicitation dialog rather than offering options that are guaranteed to fail. That check is necessarily narrow, though: the hosted MCP endpoint itself authenticates with its own access token (not a Mapboxpk.*/sk.*/tk.*token), which also lackstokens:writebut isn't detectable from its shape. For that case, "create"/"auto-create" still fail against the Tokens API, but the error now includes a scope/permission hint pointing back to "provide an existing token" instead of a bare API error.create_token_toolis disabled entirely on the hosted endpoint (unrelated to this PR).test/integration/elicitationOverHttp.test.ts) - every other elicitation test fakestool['server']directly; this one spins up an actual Streamable HTTPMcpServerand drives it with a realClientthat answerselicitation/createrequests, covering the tk.* trim, the non-tk.*-shaped-token/scope-hint path, and a full auto-create success, for both tools.httpRequestis still mocked, so this stays fully offline.McpServerper HTTP request - the same "stateless" pattern hosted-mcp-server's ownsrc/routes/mcp.tsuses (sessionIdGenerator: undefined, fresh server per POST) - and every elicitation call failed with "client does not support elicitation" regardless of what the client declared.Server#getClientCapabilities()is only ever populated on whicheverServerinstance processes the client'sinitializerequest; a fresh server per request means thetools/callrequest lands on an instance that never saw that handshake. If hosted-mcp-server's production deployment works this way, elicitation may not be functional there at all, independent of the tk.* issue above - that's hosted-mcp-server's architecture, not this PR's tool code, so it's just documented in the test's doc comment rather than addressed here.The Security Problem
Previously,
preview_style_toolandstyle_comparison_toolrequired users to provide anaccessTokendirectly in the tool input, which created security and UX risks:The Solution: Elicitation with Least Privilege
This PR implements MCP elicitation to enforce the principle of least privilege:
How It Works
When a user calls
preview_style_toolorstyle_comparison_toolwithout providing anaccessToken:Key Security Properties:
Testing
Test Case 1: First Time Preview (Elicitation Shown)
Steps:
preview_style_toolwith onlystyleId(noaccessToken)Expected:
Screenshot locations:
Test Case 2: Subsequent Preview (Cached Token)
Steps:
preview_style_toolagain with different styleIdaccessTokenprovidedExpected:
Test Case 3: Force New Token Selection
Steps:
preview_style_toolwithuseCustomToken: trueExpected:
Test Case 4: Create Token with URL Restrictions
Steps:
preview_style_toolwith new styleId,useCustomToken: trueExpected:
Test Case 5: Backward Compatibility
Steps:
preview_style_toolwith bothstyleIdANDaccessTokenExpected:
Test Case 6: Elicitation Cancellation
Steps:
preview_style_toolwithoutaccessTokenExpected:
Test Case 7: Works in Cursor
Test Case 8: Works in VS Code
Test Case 9: Works in Claude Desktop without support for Elicitation
MCP Inspector Testing
To test in MCP Inspector:
Then:
preview_style_toolorstyle_comparison_toolin tools list{"styleId": "cmi189f9600lj01sc7evj2vhs"}or{"before": "mapbox/streets-v12", "after": "mapbox/outdoors-v12"}Key Features
🔐 Security Improvements
styles:read,styles:tiles,fonts:read🎯 User Experience
accessTokenparameteruseCustomToken: trueforces new token dialog🛠️ Implementation Details
New Files:
src/utils/tokenElicitation.ts- Elicitation dialog, session-token storage, and shared HTTP helpers (listPublicPreviewTokens,createPreviewToken,isTemporaryServerToken) used by both toolstest/utils/tokenElicitation.test.ts- Unit tests for token storage, the shared HTTP helpers, and elicitation-dialog trimming (22 tests)Modified Files:
src/tools/preview-style-tool/PreviewStyleTool.ts- Integrated elicitation flow with capability checks; now takeshttpRequestvia constructor (see Update below)src/tools/preview-style-tool/PreviewStyleTool.input.schema.ts- MadeaccessTokenoptional, addeduseCustomTokensrc/tools/style-comparison-tool/StyleComparisonTool.ts- Integrated elicitation flow with capability checks; same constructor changesrc/tools/style-comparison-tool/StyleComparisonTool.schema.ts- MadeaccessTokenoptional, addeduseCustomTokensrc/tools/toolRegistry.ts,src/tools/index.ts- Updated to passhttpRequestinto both tools' constructorstest/tools/preview-style-tool/PreviewStyleTool.test.ts- Added elicitation behavior tests, including the tk.* dialog-trimming test (3 tests)test/tools/style-comparison-tool/StyleComparisonTool.test.ts- Added elicitation behavior tests, including the tk.* dialog-trimming test (3 tests)test/security/path-traversal.test.ts- Updated for the constructor signature changeREADME.md,CHANGELOG.md- Updated documentation (security model, hosted-endpoint limitation)Test Results
main, reworked to use the sharedHttpPipelineinstead of rawfetch, and gained a real HTTP integration test; see Update)Breaking Changes
None for MCP tool callers - This is fully backward compatible over the wire:
accessTokenworks exactly as beforeaccessTokenis omittedOne for direct package consumers:
PreviewStyleToolandStyleComparisonToolnow both needhttpRequestin their constructor (new PreviewStyleTool({ httpRequest })) instead of a no-arg constructor, to route the new token-listing/creation calls through the sharedHttpPipelinerather than a barefetch. The package's own pre-configured exports (import { previewStyle, styleComparison } from '@mapbox/mcp-devkit-server/tools') are updated accordingly and need no changes from consumers; only code that constructs these two classes directly is affected.Security Model
What We Protect:
Acceptable Security Trade-off: